Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

First step to install cxx_module #92

Merged
merged 7 commits into from
Nov 28, 2024
Merged

First step to install cxx_module #92

merged 7 commits into from
Nov 28, 2024

Conversation

ClausKlein
Copy link
Collaborator

@ClausKlein ClausKlein commented Nov 27, 2024 •
edited
Loading

  • works with clang-19 only on Linux and macOS
  • msvc makes trouble as always
D:\a\execution26\execution26\src\beman\execution26\execution.cppm(223): fatal error C1001: Internal compiler error.
(compiler file 'D:\a\_work\1\s\src\vctools\Compiler\CxxFE\sl\p1\c\module\writer.cpp', line 6916)
 To work around this problem, try simplifying or changing the program near the locations listed above.
If possible please provide a repro here: https://developercommunity.visualstudio.com/ 
Please choose the Technical Support command on the Visual C++ 
 Help menu, or open the Technical Support help file for more information

fixes issue #93

@ClausKlein ClausKlein marked this pull request as draft November 27, 2024 12:17
@ClausKlein ClausKlein self-assigned this Nov 27, 2024
@ClausKlein ClausKlein marked this pull request as ready for review November 27, 2024 14:02
@ClausKlein
Copy link
Collaborator Author

ClausKlein commented Nov 27, 2024 •
edited
Loading

Now it install the following cmake config packages for Debug and RelWithDebInfo:

tree stagedir/lib/
stagedir/lib/
|-- cmake
|   `-- beman_execution26
|       |-- beman_execution26-config-version.cmake
|       |-- beman_execution26-config.cmake
|       |-- beman_execution26-targets-debug.cmake
|       |-- beman_execution26-targets.cmake
|       |-- cxx-modules-beman_execution26-targets-Debug.cmake
|       |-- cxx-modules-beman_execution26-targets-RelWithDebInfo.cmake
|       |-- cxx-modules-beman_execution26-targets.cmake
|       |-- src
|       |   `-- execution.cppm
|       |-- target-beman__execution26-Debug.cmake
|       `-- target-beman__execution26-RelWithDebInfo.cmake
|-- libbeman_execution26.a
`-- libbeman_execution26D.a

4 directories, 12 files

@neatudarius what do you think about that?

dietmarkuehl
dietmarkuehl reviewed Nov 28, 2024
View reviewed changes
Copy link
Collaborator

@dietmarkuehl dietmarkuehl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good. The only bit which I'm a bit concerned about is disabling g++ on Linux.

.github/workflows/linux.yml
@@ -38,7 +16,8 @@ jobs:
matrix:
# TODO: sanitizer: [debug, release, asan, usan, tsan, lsan, msan]
preset: [debug, release]
compiler: [g++-14, clang++-19]
# TODO: compiler: [g++-15, clang++-19]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

g++ is the default compiler on Linuxes. I understand that modules are only [properly] implemented with g++-15 but it seems unfortunate to remove the default compiler entirely: is it possible to setup cmake to use modules only for some compilers?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be possible, fmt, boost do so

Is g++15 available to install on ubuntu 24.04?
If so, we should try it.

On macOS, I have only g++14 which fail to compile the module code.

CMakeLists.txt
Comment on lines +14 to +17
# Note that scanning is only performed if C++20 or higher is enabled for the target. Scanning for modules in the target's
# sources belonging to file sets of type CXX_MODULES is always performed.
# not needed: set(CMAKE_CXX_SCAN_FOR_MODULES ON)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had problems without this definition. Adding it resulted in things building on MacOS. Probably this part of the code can be removed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

@dietmarkuehl dietmarkuehl merged commit 94f15a6 into bemanproject:modules Nov 28, 2024
4 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dietmarkuehl dietmarkuehl dietmarkuehl approved these changes

@camio camio Awaiting requested review from camio camio is a code owner

@neatudarius neatudarius Awaiting requested review from neatudarius neatudarius is a code owner

Assignees

@ClausKlein ClausKlein

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ClausKlein @dietmarkuehl